Codecov Report

Merging #1633 (a0a72a5) into master (2c158a1) will decrease coverage by 19.79%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #1633       +/-   ##
===========================================
- Coverage   72.08%   52.29%   -19.80%     
===========================================
  Files         130       38       -92     
  Lines        5649     2660     -2989     
  Branches      648        0      -648     
===========================================
- Hits         4072     1391     -2681     
+ Misses       1333     1081      -252     
+ Partials      244      188       -56     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c158a1...a0a72a5. Read the comment docs.

cc @guoqqqi to check the e2e.

ping @guoqqqi @LiteSun

may sync the master branch

may sync the master branch

Done 😀

Hi @liuxiran

Thanks for your detailed explanation, I update the commit please review it again.

Hi, @stu01509 I noticed that when editing the service, the upstream information that was customized at creation disappeared, which I think should be visible here.

Hi @guoqqqi

Update the commit, I change the LINE#92

cy.contains('None').should('be.visible');

Hi @guoqqqi

Update the commit, I change the LINE#92

cy.contains('None').should('be.visible');

No, What I mean is that the data upstream of the creation of the service is now lost when editing.

Hi @liuxiran

Thanks for your instruction, I add a condition in Service/Create.tsx LINE #67, the service can successful create with None upstream, but cypress still failed, according to my change, is it caused another error?

Hi @liuxiran

Thanks for your instruction, I add a condition in Service/Create.tsx LINE #67, the service can successful create with None upstream, but cypress still failed, according to my change, is it caused another error?

Hi @stu01509 , you have to deal with the confilct first, thanks

Judging from the error log, when creating a service, cypress try to find dom #nodes_0_host, but never did it. That is because we enabled None option as the default selection, the #nodes_0_host dom will hidden.

so it would be better to set the default upstream selection to Custom.

Hi @juzhiyuan @liuxiran @nic-chen, and @guoqqqi

I update the commit, and fix the ci error, please review it again, thanks.

hi @stu01509
Conflicts, please sync code from branch master.

@stu01509 Hi, I just noticed this PR, once this conflicts get resolved, I will continue reviewing it.

Ok, I will solve it later.

Deploy preview for apisix-dashboard ready!

Built with commit cfe100c

https://deploy-preview-1633--apisix-dashboard.netlify.app

Since #1784 has been merged, @stu01509 have to fix the conflicting files, then let's go on reviewing, thanks

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.13%. Comparing base (2f3717f) to head (cfe100c).
Report is 362 commits behind head on master.

@@            Coverage Diff             @@
##           master    #1633      +/-   ##
==========================================
+ Coverage   72.07%   72.13%   +0.05%     
==========================================
  Files         125      125              
  Lines        2944     2943       -1     
  Branches      711      710       -1     
==========================================
+ Hits         2122     2123       +1     
+ Misses        822      820       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.